fix stream data decoding #112

dbazhal · 2019-01-21T14:01:00Z

Related to #88 and #104
I suppose decoding complete data is better than decoding data chunks.
Would be nice to get review from @mbohlool and @saberuster

Related to #88 and #104 I suppose decoding complete data is better than decoding data chunks.

k8s-ci-robot · 2019-01-21T14:01:04Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Make dependancy adal optional

Verify-boilerplate script

dbazhal · 2019-01-23T15:03:50Z

Ok, i think I now signed the cla, hope the bot will reconcile that.

The watch code reset the version to the last found in the response. When you first list existing objects and then start watching from that resource version the existing versions are older than the version you wanted and the watch starts from the wrong version after the first restart. This leads to for example already deleted objects ending in the stream again. Fix this by setting the minimum resource version to reset from to the input resource version. As long as k8s returns all objects in order in the watch this should work. We cannot use the integer value of the resource version to order it as one should be treat the value as opaque. Closes kubernetes-client/python#700

fix watching with a specified resource version

dbazhal · 2019-01-31T18:55:20Z

@roycaihw or @micw523 or @yliaog guys, mb any of you could take a look at my pr, or #104 please? don't want to patch this in all my controllers, and with current amount of objects in crds they fail with unicode decode errors like every hour :D

micw523 · 2019-01-31T19:21:03Z

stream/ws_client.py

-                    index = data.find("\n")
+                newline_symbol = "\n"
+                if six.PY3:
+                    newline_Symbol = b"\n"


Although it does the same thing, I wonder why you applied different logic compared to Line 44. Also, be careful about your variable name: do you mean newline_symbol instead of a capitalized S?

uh-oh, my bad, fixed that. i'll test this code tomorrorow.
and it'd be great if you could judge the solution to decoding errors itself, it bothers me more than anything else :) maybe you will find solution from #104 better than this one

micw523 · 2019-01-31T19:22:42Z

Also, it doesn't seem like your CLA has been updated yet. You sure you signed it?

tx to micw comments

dbazhal · 2019-01-31T20:29:46Z

Also, it doesn't seem like your CLA has been updated yet. You sure you signed it?

thought i signed it. now signed for sure, and email/uid match. i hope bot will recognize that

``` Traceback (most recent call last): File "controller_unitproject/run.py", line 33, in <module> run() File "controller_unitproject/run.py", line 29, in run unitproject.run() File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/controller.py", line 215, in run self.resync_and_watch() File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/controller.py", line 209, in resync_and_watch from_version=last_known_resource_version File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/watcher.py", line 92, in watch self.__watch(seconds_to_out) File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/watcher.py", line 51, in __watch for event in stream: File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/kubernetes/watch/watch.py", line 130, in stream for line in iter_resp_lines(resp): File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/kubernetes/watch/watch.py", line 47, in iter_resp_lines seg = seg.decode('utf8') UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd0 in position 2047: unexpected end of data ```

dbazhal · 2019-02-01T10:25:59Z

Fixed one more bug about decoding - now inside watch.py.
All changes tested and work fine 👍

roycaihw · 2019-02-04T21:50:55Z

commented in #104 (comment). I think we should decode utf8 only if opcode is Text

feat: merging kubeconfig files

dbazhal · 2019-03-25T14:36:00Z

Hey guys, what else can I do to help with this one? @roycaihw ?

…g_for_kconfig

…_credentials_refresh Add email scope to GCP provided credential refresh

dbazhal · 2019-05-16T16:43:32Z

I dunno if it is acceptable to up threads, but it would be nice to get any answer ^_^

roycaihw · 2019-05-23T19:42:41Z

@dbazhal sorry for the late reply. I will review this pull this week

dbazhal · 2019-06-06T09:31:34Z

just another firendly reminder :)

roycaihw · 2019-06-12T02:29:56Z

@dbazhal my apologies.. was busy with k/k 1.15 freeze. Please take a look at #104 (comment). I think recv_data_frame handles fragment concatenation already, so we don't have to concatenate the messages on top of that

Fix base64 padding for kube config

Related to #88 and #104 I suppose decoding complete data is better than decoding data chunks.

tx to micw comments

``` Traceback (most recent call last): File "controller_unitproject/run.py", line 33, in <module> run() File "controller_unitproject/run.py", line 29, in run unitproject.run() File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/controller.py", line 215, in run self.resync_and_watch() File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/controller.py", line 209, in resync_and_watch from_version=last_known_resource_version File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/watcher.py", line 92, in watch self.__watch(seconds_to_out) File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/module_k8s_controller_sdk/watcher.py", line 51, in __watch for event in stream: File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/kubernetes/watch/watch.py", line 130, in stream for line in iter_resp_lines(resp): File "/home/bajal/.local/share/virtualenvs/controller_unitproject-1u3cbEWP/lib/python3.7/site-packages/kubernetes/watch/watch.py", line 47, in iter_resp_lines seg = seg.decode('utf8') UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd0 in position 2047: unexpected end of data ```

Following comment #112 (comment) Tested on running controller.

k8s-ci-robot · 2019-06-20T17:56:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dbazhal
To complete the pull request process, please assign roycaihw
You can assign the PR to them by writing /assign @roycaihw in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2019-06-20T17:56:55Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

bpicolo and others added 8 commits July 30, 2018 15:26

Fix base64 padding for kube config

529a72a

Merge branch 'master' into fix_base64_padding_for_kconfig

d54efa7

Add verify-boilerplate script

86ae2de

Verify Boilerplate fix

d56fdbc

Make dependancy adal optional

375befb

Add email scope to GCP provided credential refresh

1637d56

fix stream data decoding

989e4b3

Related to #88 and #104 I suppose decoding complete data is better than decoding data chunks.

complete data decoding

545690c

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 21, 2019

k8s-ci-robot requested review from mbohlool and roycaihw January 21, 2019 14:01

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 21, 2019

k8s-ci-robot added 2 commits January 22, 2019 18:12

Merge pull request #108 from iamneha/make-adal-optional

260f855

Make dependancy adal optional

Merge pull request #107 from iamneha/verify-scripts

8497dfb

Verify-boilerplate script

juliantaylor and others added 2 commits January 23, 2019 19:38

Merge pull request #109 from juliantaylor/fix-watch-reset

2d69e89

fix watching with a specified resource version

micw523 reviewed Jan 31, 2019

View reviewed changes

fix symbols definitions

97ad43b

tx to micw comments

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 31, 2019

dbazhal added 2 commits February 1, 2019 13:06

correct decoding inside of watch

8aaa929

Updated OWNERS to include link to docs

4d387d5

tomplus and others added 2 commits March 15, 2019 23:42

feat: merging kubeconfig files

328b2d1

Merge pull request #94 from tomplus/feat/merge-kubeconfigs

5c03b3b

feat: merging kubeconfig files

bpicolo and others added 3 commits March 30, 2019 09:23

Add tests for updated pieces

b3ddbd9

Merge remote-tracking branch 'upstream/master' into fix_base64_paddin…

72a02cc

…g_for_kconfig

Merge pull request #110 from axelsteingrimsson/add_email_scope_to_gcp…

1d5231c

…_credentials_refresh Add email scope to GCP provided credential refresh

leavest mentioned this pull request Apr 20, 2019

Fix stream decode error #130

Closed

Merge pull request #79 from bpicolo/fix_base64_padding_for_kconfig

487c918

Fix base64 padding for kube config

dbazhal mentioned this pull request Jun 20, 2019

decode add a replace option #104

Merged

dbazhal and others added 10 commits June 20, 2019 20:53

fix stream data decoding

c1a0a37

Related to #88 and #104 I suppose decoding complete data is better than decoding data chunks.

complete data decoding

6cc9fa0

fix symbols definitions

678224a

tx to micw comments

correct decoding inside of watch

fc1dc36

make ws client decode data only on OPCODE_TEXT

8a6f093

Following comment #112 (comment) Tested on running controller.

Update watch.py

9008491

Update watch_test.py

4295b2d

no need to fix that

e8db1ba

Merge branch 'master' of github.com:dbazhal/python-base

c36eada

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 20, 2019

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 20, 2019

dbazhal closed this Jun 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix stream data decoding #112

fix stream data decoding #112

dbazhal commented Jan 21, 2019

k8s-ci-robot commented Jan 21, 2019

dbazhal commented Jan 23, 2019

dbazhal commented Jan 31, 2019

micw523 Jan 31, 2019

dbazhal Jan 31, 2019 •

edited

Loading

micw523 commented Jan 31, 2019

dbazhal commented Jan 31, 2019

dbazhal commented Feb 1, 2019

roycaihw commented Feb 4, 2019

dbazhal commented Mar 25, 2019

dbazhal commented May 16, 2019

roycaihw commented May 23, 2019

dbazhal commented Jun 6, 2019

roycaihw commented Jun 12, 2019

k8s-ci-robot commented Jun 20, 2019

k8s-ci-robot commented Jun 20, 2019

fix stream data decoding #112

fix stream data decoding #112

Conversation

dbazhal commented Jan 21, 2019

k8s-ci-robot commented Jan 21, 2019

dbazhal commented Jan 23, 2019

dbazhal commented Jan 31, 2019

micw523 Jan 31, 2019

Choose a reason for hiding this comment

dbazhal Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

micw523 commented Jan 31, 2019

dbazhal commented Jan 31, 2019

dbazhal commented Feb 1, 2019

roycaihw commented Feb 4, 2019

dbazhal commented Mar 25, 2019

dbazhal commented May 16, 2019

roycaihw commented May 23, 2019

dbazhal commented Jun 6, 2019

roycaihw commented Jun 12, 2019

k8s-ci-robot commented Jun 20, 2019

k8s-ci-robot commented Jun 20, 2019

dbazhal Jan 31, 2019 •

edited

Loading